fix(arith): bug in vartime inversion when using fused inverse+multiply by factor - found by Guido Vranken #433
+29
−2
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Constantine implements a fused inversion+multiply (FIM) that does
FIM(a, F, p) = F.a⁻¹ (mod p)
This allows inversion in the Montgomery domain without converting back-and-forth in the canonical representation.
As a reminder, in the Montgomery domain we map a to a' with
a' = aR (mod p)
with R a magic constant, for example forFr[BLS12_381]
which is 255 bits,R = 2⁶⁴*⁴ (mod p)
i.e. the next multiple of the word size that can fully store the modulus p.Hence
a'⁻¹ = a⁻¹R (mod p) = (aR)⁻¹.R² (mod p) = FIM(a', R², p)
Unfortunately the code was shortcutting when
a' == 1
to 1 instead of F.constantine/constantine/math/arithmetic/limbs_exgcd.nim
Lines 844 to 858 in 9a2d23b
Impact
Fortunately at the protocol-level Constantine only uses vartime inversion in MSM and KZG:
In particular BLS signatures do not use
inv_vartime
infinalExpEasy
cc @guidovranken